[Observability AI Assistant] migrate to inference client #197630 #199286

arturoliduena · 2024-11-07T10:32:03Z

Summary

Closes #197630
[Observability AI Assistant] Partially migrate to inference client

replacing inferenceClient.chatComplete to observabilityAIAssistantClient.chat - observabilityAIAssistantClient.complete does a bunch of stuff on top of chat. keepping observabilityAIAssistantClient.chat as a wrapper for now because it also adds instrumentation and logging.

elasticmachine · 2024-11-07T10:32:05Z

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

github-actions · 2024-11-07T10:32:16Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

dgieselaar · 2024-11-09T16:47:56Z

x-pack/plugins/observability_solution/observability_ai_assistant/server/service/client/index.ts

-              throw createInternalServerError(
-                `${executeResult?.message} - ${executeResult?.serviceMessage}`
-              );
+  ): Observable<ChatCompletionEvent> => {


Ideally we encapsulate the event type from the inference client here, and convert it into ChatCompletionChunkEvent. We should eventually completely drop ChatCompletionChunkEvent in favor of the inference plugin's types, but that means that we need to make changes in many places, and there's some stuff that isn't supported yet by the Inference plugin's types. Until that happens, I think we should limit the blast radius and stick to our own types where we can - which should be everywhere except for the implementation of this function, I think.

agree, I will use ChatEvent to maintain compatibility with existing types (ChatCompletionChunkEvent, and TokenCountEvent) and adding InferenceChatCompletionChunkEvent

To clarify, what I mean is that we only have ChatCompletionChunkEvent (which is exclusive to the Obs AI Assistant). Any event from the Inference client is ideally not exposed via the chat method. So the signature of the chat method should stay the same, and existing types as well. Does that make sense?

Yes, that makes sense. I’ll keep the ChatCompletionChunkEvent as the main event type for the ObservabilityAIAssistantClient.chat method to ensure we don’t expose any inference-client-specific types directly. I’ll handle the conversion internally within the implementation, encapsulating the inference client events as needed without altering the existing method signature or exposing the InferenceChatCompletionChunkEvent.

Solution:
Within the pipe, we’ll add a mapping step to convert any incoming InferenceChatCompletionChunkEvent to a ChatCompletionChunkEvent type. should I do it for all the event emitted from inferenceClient.complete method?
for these 3 inference client events:

export type ChatCompletionEvent<TToolOptions extends ToolOptions = ToolOptions> = | ChatCompletionChunkEvent | ChatCompletionTokenCountEvent | ChatCompletionMessageEvent<TToolOptions>;

arturoliduena · 2024-11-13T11:13:36Z

x-pack/plugins/observability_solution/observability_ai_assistant/server/service/client/index.ts

+        tools,
+      })
+    ).pipe(
+      convertInferenceEventsToStreamingEvents(),


@dgieselaar
convertInferenceEventsToStreamingEvents converts any InferenceChatCompletionEvent types to their corresponding StreamingChatResponseEvent types, such as ChatCompletionChunkEvent, TokenCountEvent, ensuring we maintain compatibility with existing event types. This keeps the ObservabilityAIAssistantClient.chat method’s signature intact and avoids exposing inference-client-specific types directly.

++, this looks good!

x-pack/plugins/observability_solution/observability_ai_assistant/server/service/client/index.ts

x-pack/test/observability_ai_assistant_api_integration/common/create_openai_chunk.ts

afharo

kibana.jsonc LGTM

sorenlouv · 2024-12-03T10:44:52Z

x-pack/test/observability_ai_assistant_api_integration/tests/chat/chat.spec.ts

            }

+            await simulator.tokenCount({ completion: 20, prompt: 33, total: 53 });


Did you manually calculate these? If the input chunks are changed, will the token counts go out of sync without us noticing? Theres nothing that actually counts the tokens and require the token count to match?

…I Assistant client

…ken tracking

…ion calls in AI Assistant tests

…ution

…ssistant/tests

…create_openai_chunk.ts Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>

…chunk utility

…nt/server/service/client/operators/convert_inference_events_to_streaming_events.ts Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>

…vents and simplify token count extraction

elasticmachine · 2024-12-09T08:55:11Z

💚 Build Succeeded

Buildkite Build
Commit: 2355ca4
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-199286-2355ca46d51c

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`observabilityAIAssistant`	294	296	+2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`observabilityAIAssistant`	48.1KB	48.3KB	+206.0B

Unknown metric groups

API count

id	before	after	diff
`observabilityAIAssistant`	296	298	+2

History

💛 Build #256928 was flaky 1040c29
💔 Build #256812 failed 8190ba8
💚 Build #256550 succeeded cfff370
💔 Build #256418 failed 2bee2f4
💚 Build #256334 succeeded 578d51e

kibanamachine · 2024-12-09T11:46:50Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12234966215

… (elastic#199286) ## Summary Closes elastic#183245 Closes elastic#197630 [Observability AI Assistant] Partially migrate to inference client replacing `inferenceClient.chatComplete` to `observabilityAIAssistantClient.chat` - `observabilityAIAssistantClient.complete` does a bunch of stuff on top of `chat`. keepping `observabilityAIAssistantClient.chat` as a wrapper for now because it also adds instrumentation and logging. (cherry picked from commit df0dfa5)

kibanamachine · 2024-12-09T11:51:48Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

#199286) (#203399) # Backport This will backport the following commits from `main` to `8.x`: - [[Observability AI Assistant] migrate to inference client #197630 (#199286)](#199286)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Arturo Lidueña <arturo.liduena@elastic.co>

… (elastic#199286) ## Summary Closes elastic#183245 Closes elastic#197630 [Observability AI Assistant] Partially migrate to inference client replacing `inferenceClient.chatComplete` to `observabilityAIAssistantClient.chat` - `observabilityAIAssistantClient.complete` does a bunch of stuff on top of `chat`. keepping `observabilityAIAssistantClient.chat` as a wrapper for now because it also adds instrumentation and logging.

arturoliduena added Team:Obs AI Assistant Observability AI Assistant backport:version Backport to applied version labels v8.17.0 labels Nov 7, 2024

arturoliduena requested review from a team as code owners November 7, 2024 10:32

botelastic bot added the ci:project-deploy-observability Create an Observability project label Nov 7, 2024

arturoliduena force-pushed the 197630-o11y-ai-assistant-inferenceClient-migration branch 2 times, most recently from 60177d0 to a3b2644 Compare November 7, 2024 14:46

pgayvallet requested a review from a team November 8, 2024 07:22

arturoliduena marked this pull request as draft November 8, 2024 08:34

arturoliduena added the release_note:skip Skip the PR/issue when compiling release notes label Nov 8, 2024

dgieselaar reviewed Nov 9, 2024

View reviewed changes

arturoliduena force-pushed the 197630-o11y-ai-assistant-inferenceClient-migration branch 3 times, most recently from c59ff8b to df2a179 Compare November 13, 2024 11:04

arturoliduena commented Nov 13, 2024

View reviewed changes

arturoliduena force-pushed the 197630-o11y-ai-assistant-inferenceClient-migration branch from df2a179 to 200217b Compare November 26, 2024 09:24

arturoliduena marked this pull request as ready for review November 26, 2024 09:26

arturoliduena requested a review from a team as a code owner November 26, 2024 10:13

sorenlouv reviewed Nov 26, 2024

View reviewed changes

x-pack/plugins/observability_solution/observability_ai_assistant/server/service/client/index.ts Show resolved Hide resolved

sorenlouv reviewed Nov 26, 2024

View reviewed changes

x-pack/test/observability_ai_assistant_api_integration/common/create_openai_chunk.ts Outdated Show resolved Hide resolved

afharo approved these changes Nov 26, 2024

View reviewed changes

arturoliduena force-pushed the 197630-o11y-ai-assistant-inferenceClient-migration branch 5 times, most recently from ef466d1 to 578d51e Compare December 3, 2024 09:44

sorenlouv reviewed Dec 3, 2024

View reviewed changes

arturoliduena and others added 13 commits December 9, 2024 08:55

Enhance chat completion handling and update tests for Observability A…

245474d

…I Assistant client

Add tokenCount method to LlmResponseSimulator and update tests for to…

206baa8

…ken tracking

Refactor conversation simulator to use tool_calls structure for funct…

e0821e0

…ion calls in AI Assistant tests

Fix tokenCount method call in chat.spec.ts to await asynchronous exec…

e8094ec

…ution

update test_serverless/api_integration/test_suites/observability/ai_a…

494aded

…ssistant/tests

Add tokenCount calls to conversation and title simulators in API tests

04670ff

Update x-pack/test/observability_ai_assistant_api_integration/common/…

7101f7d

…create_openai_chunk.ts Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>

Simplify CreateChatCompletionResponseChunk type definition in OpenAI …

f044957

…chunk utility

Update x-pack/plugins/observability_solution/observability_ai_assista…

e54706a

…nt/server/service/client/operators/convert_inference_events_to_streaming_events.ts Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>

Update x-pack/plugins/observability_solution/observability_ai_assista…

e30e0d9

…nt/server/service/client/operators/convert_inference_events_to_streaming_events.ts Co-authored-by: Søren Louv-Jansen <sorenlouv@gmail.com>

Import Observable in convert_inference_events_to_streaming_events.ts

74d3db8

Refactor event conversion logic in convertInferenceEventsToStreamingE…

85e4cfb

…vents and simplify token count extraction

Add toolCalls to InferenceChatCompletionEvent in LLM simulator

2355ca4

arturoliduena force-pushed the 197630-o11y-ai-assistant-inferenceClient-migration branch from 1040c29 to 2355ca4 Compare December 9, 2024 07:55

arturoliduena added v8.18.0 and removed v8.17.0 labels Dec 9, 2024

arturoliduena merged commit df0dfa5 into elastic:main Dec 9, 2024
11 checks passed

kibanamachine added the v9.0.0 label Dec 9, 2024

kibanamachine mentioned this pull request Dec 9, 2024

[8.x] [Observability AI Assistant] migrate to inference client #197630 (#199286) #203399

Merged

arturoliduena mentioned this pull request Dec 11, 2024

[Obs AI Assistant] Fix null pointer in function definition #203344

Merged

sorenlouv mentioned this pull request Feb 4, 2025

[Obs AI Assistant] System message is missing #209548

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Observability AI Assistant] migrate to inference client #197630 #199286

[Observability AI Assistant] migrate to inference client #197630 #199286

arturoliduena commented Nov 7, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

dgieselaar Nov 9, 2024

arturoliduena Nov 11, 2024

dgieselaar Nov 12, 2024

arturoliduena Nov 12, 2024

arturoliduena Nov 13, 2024

dgieselaar Nov 13, 2024

afharo left a comment

sorenlouv Dec 3, 2024 •

edited

Loading

elasticmachine commented Dec 9, 2024 •

edited

Loading

API count

kibanamachine commented Dec 9, 2024

kibanamachine commented Dec 9, 2024

		}

		await simulator.tokenCount({ completion: 20, prompt: 33, total: 53 });

[Observability AI Assistant] migrate to inference client #197630 #199286

[Observability AI Assistant] migrate to inference client #197630 #199286

Conversation

arturoliduena commented Nov 7, 2024 • edited by kibanamachine Loading

Summary

elasticmachine commented Nov 7, 2024

github-actions bot commented Nov 7, 2024

🤖 GitHub comments

dgieselaar Nov 9, 2024

Choose a reason for hiding this comment

arturoliduena Nov 11, 2024

Choose a reason for hiding this comment

dgieselaar Nov 12, 2024

Choose a reason for hiding this comment

arturoliduena Nov 12, 2024

Choose a reason for hiding this comment

arturoliduena Nov 13, 2024

Choose a reason for hiding this comment

dgieselaar Nov 13, 2024

Choose a reason for hiding this comment

afharo left a comment

Choose a reason for hiding this comment

sorenlouv Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Dec 9, 2024 • edited Loading

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Page load bundle

API count

History

kibanamachine commented Dec 9, 2024

kibanamachine commented Dec 9, 2024

💚 All backports created successfully

Questions ?

arturoliduena commented Nov 7, 2024 •

edited by kibanamachine

Loading

sorenlouv Dec 3, 2024 •

edited

Loading

elasticmachine commented Dec 9, 2024 •

edited

Loading